Skip to content

Support decryption with hidden recipients. #1275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
duckdalbe opened this issue Aug 25, 2020 · 10 comments · Fixed by #1940
Closed

Support decryption with hidden recipients. #1275

duckdalbe opened this issue Aug 25, 2020 · 10 comments · Fixed by #1940
Assignees
Milestone

Comments

@duckdalbe
Copy link

RNP reports that it cannot decrypt if no Key ID of the Public-Key Encrypted Session Key Packets matches a known private key.

It would be great if it instead could check for "hidden" recipients. In case the Key ID 0x00000000 is present, RNP should try to decrypt with all known private keys. See https://tools.ietf.org/html/rfc4880#section-5.1 (last paragraph of that section).

Background: Currently Thunderbird 78 an later fails to decrypt messages where the user is a hidden recipient. Such ciphertexts could e.g. be created by Enigmail, or in general with GnuPG's hidden-encrypt-to config option. Downstream issue: https://thunderbird.topicbox.com/groups/e2ee/T7e31aaa38399d4b3-Mefc2734288f0dfa17d01b823/fails-to-decrypt-with-recipient-key-id-0x00000000-hidden-recipient.

@ni4
Copy link
Contributor

ni4 commented Aug 25, 2020

Thanks for reporting, we'll add corresponding functionality once get to this issue.

@ni4 ni4 self-assigned this Aug 25, 2020
@ni4 ni4 added this to the v0.15.0 milestone Jan 18, 2021
@ni4 ni4 modified the milestones: v0.15.0, v0.16.0 Mar 9, 2021
@ronaldtse ronaldtse moved this to URGENT in RNP Dec 17, 2021
@ronaldtse ronaldtse added this to RNP Dec 17, 2021
@ni4 ni4 modified the milestones: v0.16.0, 0.17.0 Jan 6, 2022
@andrey-utkin
Copy link
Contributor

I'd be interested to look into this.
Seems clear how to write a test for it, and then the test will guide my investigation.

@ni4 ni4 assigned andrey-utkin and unassigned ni4 Mar 3, 2022
@ni4
Copy link
Contributor

ni4 commented Mar 3, 2022

@andrey-utkin Feel free to take it!
The only thing is that I'd prefer to make this feature configurablte via the SecurityContext, so user may enable or disable this.
Feel free to ask additional questions if you have.

@andrey-utkin
Copy link
Contributor

Sorry for the slow start.

Test so far:

# - create a keypair in rnp
rnpkeys --generate-key --password '' --userid 1275@rnp

# - export pubkey from rnp
rnpkeys --export-key --userid 1275@rnp > rnp.pubkey.asc

# - import pubkey to gnupg
gpg --import < rnp.pubkey.asc

# - encrypt using the pubkey with gnupg with hidden-encrypt-to option
echo hello | gpg --trust-model always --armor  --recipient 1275@rnp --encrypt > present-to.pgp-msg.asc
echo hello | gpg --trust-model always --armor  --hidden-recipient 1275@rnp --encrypt > hidden-to.pgp-msg.asc

# - decrypt this with rnp
rnp --decrypt present-to.pgp-msg.asc # works, prints "hello"

# https://github.com/rnpgp/rnp/issues/1275
# [init_encrypted_src() /var/tmp/portage/app-crypt/rnp-9999/work/rnp-9999/src/librepgp/stream-parse.cpp:2163] failed to obtain decrypting key or password
# exit code 1
rnp --decrypt hidden-to.pgp-msg.asc

I will proceed by investigating it and in parallel fitting the test into some existing test suite.
Pythonic one seems to be well-suited for such things, is that right?

@ni4
Copy link
Contributor

ni4 commented Mar 8, 2022

I will proceed by investigating it and in parallel fitting the test into some existing test suite.
Pythonic one seems to be well-suited for such things, is that right?

I'd prefer to have FFI API test as well, since people who use library (like Thunderbird) could use approach which differs from a CLI one. Also it would be good to have test for different cases, like alread-preloaded secret keys, keys loaded by demand via key provider, trying multiple secret keys and so on.

@andrey-utkin
Copy link
Contributor

I got it working with the clumsiest hack ever. Cleaning up and refactoring now, will submit into a branch later.
The meaning of these hooks becomes less certain with this extension of functionality in mind:

typedef void pgp_on_recipients_func_t(const std::vector<pgp_pk_sesskey_t> &recipients,
                                      const std::vector<pgp_sk_sesskey_t> &passwords,
                                      void *                               param);
typedef void pgp_decryption_start_func_t(pgp_pk_sesskey_t *pubenc,
                                         pgp_sk_sesskey_t *symenc,
                                         void *            param);

on_recipients() gets zeros-filled entries. Making it see the effective secret keys would take some extra care, but it's unclear whether anybody really needs that extra complexity.
pgp_decryption_start() is supposed to receive the "pubenc" packet corresponding to the decryption key used. But it will either hold zeros instead of effective key id, or have a patched pubenc which is not verbatim and refers to a key which potentially wasn't in on_recipients() data.

My judgement is that optimal behaviour is

  • on_recipients() gets verbatim pubencs list, including zeros
  • pgp_decryption_start() gets pubenc patched with the effective keyid

@ni4
Copy link
Contributor

ni4 commented Mar 23, 2022

@andrey-utkin Good news! Agree with your approach: on_recipients() should have original recipient list (so implementation which uses corresponding FFI API may see that original recipient was hidden), and used_recipient should be set to the key data which was used for the decryption.

@andrey-utkin
Copy link
Contributor

This is my branch so far: https://github.com/andrey-utkin/rnp/tree/autkin-1275-decrypt-hidden-to
History will be cleaned up for PR submission.
FFI test is underway.
Will look into "make this feature configurablte via the SecurityContext" next and add tests for that.

@ni4
Copy link
Contributor

ni4 commented Mar 26, 2022

@andrey-utkin Great! Could you please create a draft PR from your branch? It would be easier to discuss changes there (and would run CI including linters and so on)?

@andrey-utkin
Copy link
Contributor

Created a draft PR as requested: #1792

andrey-utkin added a commit to andrey-utkin/rnp that referenced this issue Apr 12, 2022
To decrypt messages with hidden recipients (per rnpgp#1275) we need to
attempt decrypting with every available private key capable of
encryption. Attempts with non-corresponding keys will obviously fail,
but that is not a failure from user's sperspective, so there should not
be log messages about decryption failures.

This requirement has been partially satisfied by passing a `bool
speculative` parameter. However it doesn't seem appropriate to pass it
across the boundary of pluggable crypto backend. And then there's
currently no other context passed across that boundary - neither
rnp_ffi_t nor SecurityContext.

This commit simply drops one message statement from OpenSSL crypto
backend to avoid a larger structural rework.
@ni4 ni4 assigned ni4 and unassigned andrey-utkin Jun 14, 2022
Repository owner moved this from URGENT to Done in RNP Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants